Move health check to the backend #2120
Conversation
| */ | ||
| async testDatasource() { | ||
| const backendDS = new DataSourceWithBackend(this.instanceSettings); | ||
| try { |
There was a problem hiding this comment.
For this, I am using this approach for now, but it should be temporary, I have a separate issue to track migrating from DatasourceApi to DatasourceWithBackend when this is complete we will no longer need to have this here.
itsmylife
left a comment
itsmylife
left a comment
There was a problem hiding this comment.
It looks good but we need to double check if we still need testDatasource()
| /** | ||
| * Test connection to Zabbix API and external history DB. | ||
| */ | ||
| async testDatasource() { |
There was a problem hiding this comment.
Do we still need this method? I think this is needed for frontend implementation. Now the config editor must have called backend CheckHealth endpoint out of the box.
There was a problem hiding this comment.
I thought we still needed it. Maybe once we migrate to DatasourceWithBackend (in progress) we don't, but even then, for this scenario we will still need it, because the part that tests the DB connection cannot be moved to the backend, in the frontend we call those datasource's "testDatasource" and they call their backend, and so here we merge the results from both.
I also checked a few other datasources and can see that they are still using the testDatasource method, for example: jaeger and tempo
There was a problem hiding this comment.
They were frontend only data sources and migrated to backend. Actually I raised that question in some other place too. In prometheus we don't use it. Do you mind checking what happens when we remove that method?
zoltanbedi
changed the title
This PR moves the health check to backend only leaving in the frontend the functionality to test the dbconnector datasource.
Leaving the
dbconnector.testDataSourceshould be fine since the datasource types we allow for db connection with Zabbix already are backend datasources, and so their health requests would go through the backend.Verified:
Clicking test and seeing a
healthrequest go out.IMPORTANT: While testing this in the UI, I found a bug with the config editor - whenever a change is made in the UI and tested, the changes don't take effect (i.e. disabling trends, keeps
trendsset totrue, enabling db connection keepdbConnectionEnabledset tofalseand so on.). Created a separate issue to fix thisFixes https://github.com/grafana/oss-big-tent-squad/issues/124
Fixes #2004